Skip to content

Conversation

@ozdemirburak
Copy link

Added the optional autoplay option to stopwatch.

Sorry for the second commit as I, by default, use 2 spaces for indent :)

jquery.timer.js Outdated
this.pauseStart = 0;
this.setInterval();

if (autostart != 0) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be simplified to just if (autostart)

Copy link
Author

@ozdemirburak ozdemirburak Jan 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually that line is there to keep autostart as the default behavior as currently stopwatch starts automatically? Because if someone does not pass the autostart variable within the constructor, then it won't start by default as the autostart variable will be undefined.

But anyway, changing it.

};

/**
* @param {function=} updateFunction
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind updating the docblock? (also for the set function below)

@jchavannes
Copy link
Owner

@ozdemirburak Thanks for the PR! I'd be glad to add this. I left a few minor comments. Also, would you mind squashing your commits since all the changed lines will lose Git history (i.e. https://github.com/ozdemirburak/jquery-timer/blame/a6b47518ffc3827a774ed40d739b588ee80b047b/jquery.timer.js#L31).

@ozdemirburak
Copy link
Author

@jchavannes ping :)

@jchavannes jchavannes merged commit 83142dd into jchavannes:stopwatch Mar 29, 2017
@jchavannes
Copy link
Owner

Merged. Thanks again @ozdemirburak :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants